btcwallet: Evaluate the properness of `GetTransaction` API confirmations patch #10364

mohamedawnallah · 2025-11-13T13:05:30Z

Objectives

The goal of this PR is to make sure there are no unintended consequences of applying that btcwallet patch. This can be explicitly verified by making sure there are no relevant tests failing in the CI.

Motivation and Context

Follow-up on @yyforyongyu's comment btcsuite/btcwallet#1114 (review).

Triggering Points

This is the only place in LND codebase where Confirmations field from GetTransaction API is conditionally utilized:

lnd/lnwallet/btcwallet/btcwallet.go

Lines 1292 to 1329 in af6816b

    
           // GetTransactionDetails returns details of a transaction given its 
        
           // transaction hash. 
        
           func (b *BtcWallet) GetTransactionDetails( 
        
           	txHash *chainhash.Hash) (*lnwallet.TransactionDetail, error) { 
        
           	// Grab the best block the wallet knows of, we'll use this to calculate 
        
           	// # of confirmations shortly below. 
        
           	bestBlock := b.wallet.SyncedTo() 
        
           	currentHeight := bestBlock.Height 
        
           	tx, err := b.wallet.GetTransaction(*txHash) 
        
           	if err != nil { 
        
           		return nil, err 
        
           	} 
        
           	// For both confirmed and unconfirmed transactions, create a 
        
           	// TransactionDetail which re-packages the data returned by the base 
        
           	// wallet. 
        
           	if tx.Confirmations > 0 { 
        
           		txDetails, err := minedTransactionsToDetails( 
        
           			currentHeight, 
        
           			base.Block{ 
        
           				Transactions: []base.TransactionSummary{ 
        
           					tx.Summary, 
        
           				}, 
        
           				Hash:      tx.BlockHash, 
        
           				Height:    tx.Height, 
        
           				Timestamp: tx.Summary.Timestamp}, 
        
           			b.netParams, 
        
           		) 
        
           		if err != nil { 
        
           			return nil, err 
        
           		} 
        
           		return txDetails[0], nil 
        
           	} 
        
           	return unminedTransactionsToDetail(tx.Summary, b.netParams) 
        
           }

Pull Request Checklist

Testing

Your PR passes all CI checks.
Tests covering the positive and negative (error paths) are included.
Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

The change is not insubstantial. Typo fixes are not accepted to fight bot spam.
The change obeys the Code Documentation and Commenting guidelines, and lines wrap at 80.
Commits follow the Ideal Git Commit Structure.
Any new logging statements use an appropriate subsystem and logging level.
Any new lncli commands have appropriate tags in the comments for the rpc in the proto file.
There is a change description in the release notes, or [skip ci] in the commit message for small changes.

📝 Please see our Contribution Guidelines for further guidance.

gemini-code-assist · 2025-11-13T13:05:53Z

Summary of Changes

Hello @mohamedawnallah, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on validating a specific patch to the btcwallet's GetTransaction API confirmations. To achieve this, the PR updates a wide array of Go module dependencies to their most recent versions and includes a temporary replace directive for btcwallet. The core objective is to confirm that these updates and the btcwallet patch do not introduce any regressions or unintended side effects, primarily verified by the successful execution of CI tests.

Highlights

Dependency Updates: Numerous Go module dependencies have been updated to their latest versions, including core btcsuite components, golang.org/x libraries, and google.golang.org/grpc.
Temporary btcwallet Replacement: A replace directive has been added in go.mod to temporarily point github.com/btcsuite/btcwallet to a specific fork by the author, github.com/mohamedawnallah/btcwallet.
Patch Evaluation: The primary goal of these changes is to evaluate the properness of a btcwallet confirmations patch for the GetTransaction API, ensuring that the updated dependencies do not introduce unintended consequences or regressions.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request updates numerous dependencies and, most notably, adds a replace directive in go.mod to use a personal fork of btcwallet. This is presumably to test a patch for btcwallet via CI. My review contains a critical comment regarding this replace directive. Using a personal fork for a core dependency introduces significant security and maintainability risks and should not be merged into a main branch. The patch should be merged upstream into the official repository instead.

go.mod

btcwallet: evaluate GetTransaction confirmations proper patch

40d09a9

mohamedawnallah mentioned this pull request Nov 13, 2025

wallet: resolve root cause of broken zero-conf and inflated confirmations in GetTransaction API btcsuite/btcwallet#1114

Merged

7 tasks

gemini-code-assist bot reviewed Nov 13, 2025

View reviewed changes

go.mod Show resolved Hide resolved

yyforyongyu added this to the v0.21.0 milestone Nov 17, 2025

yyforyongyu added this to v0.21 Nov 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

btcwallet: Evaluate the properness of `GetTransaction` API confirmations patch #10364

btcwallet: Evaluate the properness of `GetTransaction` API confirmations patch #10364

Uh oh!

mohamedawnallah commented Nov 13, 2025

Uh oh!

gemini-code-assist bot commented Nov 13, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	// GetTransactionDetails returns details of a transaction given its
	// transaction hash.
	func (b *BtcWallet) GetTransactionDetails(
	txHash chainhash.Hash) (lnwallet.TransactionDetail, error) {

	// Grab the best block the wallet knows of, we'll use this to calculate
	// # of confirmations shortly below.
	bestBlock := b.wallet.SyncedTo()
	currentHeight := bestBlock.Height
	tx, err := b.wallet.GetTransaction(*txHash)
	if err != nil {
	return nil, err
	}

	// For both confirmed and unconfirmed transactions, create a
	// TransactionDetail which re-packages the data returned by the base
	// wallet.
	if tx.Confirmations > 0 {
	txDetails, err := minedTransactionsToDetails(
	currentHeight,
	base.Block{
	Transactions: []base.TransactionSummary{
	tx.Summary,
	},
	Hash: tx.BlockHash,
	Height: tx.Height,
	Timestamp: tx.Summary.Timestamp},
	b.netParams,
	)
	if err != nil {
	return nil, err
	}

	return txDetails[0], nil
	}

	return unminedTransactionsToDetail(tx.Summary, b.netParams)
	}

btcwallet: Evaluate the properness of GetTransaction API confirmations patch #10364

Are you sure you want to change the base?

btcwallet: Evaluate the properness of GetTransaction API confirmations patch #10364

Uh oh!

Conversation

mohamedawnallah commented Nov 13, 2025

Objectives

Motivation and Context

Triggering Points

Pull Request Checklist

Testing

Code Style and Documentation

Uh oh!

gemini-code-assist bot commented Nov 13, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

btcwallet: Evaluate the properness of `GetTransaction` API confirmations patch #10364

btcwallet: Evaluate the properness of `GetTransaction` API confirmations patch #10364